Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add bottleneck function #145

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

feat: add bottleneck function #145

wants to merge 2 commits into from

Conversation

aleclarson
Copy link
Member

@aleclarson aleclarson commented Jul 29, 2024

Tip

The owner of this PR can publish a preview release by commenting /publish in this PR. Afterwards, anyone can try it out by running pnpm add radashi@pr<PR_NUMBER>.

Summary

Limit how many times a function can be called per time interval (in milliseconds). Optionally set a concurrency limit.

Most useful for rate limiting.

Related issue, if any:

lodash/lodash#2102 (cc @not-an-aardvark from there)

For any code change,

  • Related documentation has been updated, if needed
  • Related tests have been added or updated, if needed
  • Related benchmarks have been added or updated, if needed

Does this PR introduce a breaking change?

No

Bundle impact

Status File Size 1
A src/curry/bottleneck.ts 590

Footnotes

  1. Function size includes the import dependencies of the function.

@Minhir
Copy link
Member

Minhir commented Jul 30, 2024

What do you think about 'ratelimit' name to make it more explicit?

@aleclarson
Copy link
Member Author

What do you think about 'ratelimit' name to make it more explicit?

Not against the idea. The name bottleneck is a nod to bottleneck on NPM.

Claude Sonnet 3.5 seemed to like both names 😄

// every finished call.
const next = () => queue.length && run(queue.shift()!)

const bottled: BottledFunction<TArgs, any> = (...args) => run(args)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can create a QueueItem here, push it in the queue and call next(). That should help us to simplify run function to run(input: QueueItem).

Something like this:

const bottled: BottledFunction<TArgs, any> = (...args) => {
  const { promise, resolve, reject } = Promise.withResolvers();
  const item = { args, resolve, reject };
  
  queue.push(item);
  next();
  
  return promise;
}

Probably we need to create our internal withResolvers util since that API is very new.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we need to create our internal withResolvers util since that API is very new.

Would you be interested in contributing that?

You can do this in terminal to create the files:

pnpm add-function async/withResolvers

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on the simplification, even if it means a little more GC work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a PR #148

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW @aleclarson add-functiondoesn't put an export into src/mod.ts. (also, we can use https://github.com/google/zx instead of sh, but maybe it's overengineering)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add-functiondoesn't put an export into src/mod.ts

Yeah I hope to fix that eventually 😅

The add-function script should have warned you though. Did it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, no 🤔

$ pnpm add-function async/withResolvers

> radashi@12.1.0 add-function /home/vladimir/code/radashi
> bash ./scripts/add-function.sh "async/withResolvers"

Enter a description for withResolvers:
Ponyfill for Promise.withResolvers()

@aleclarson
Copy link
Member Author

I think rateLimit feels “higher level”, which could invite scope creep for more rate-limiting features. With enough tweaking, a rateLimit function could conceivably reimplement throttle and debounce. Maybe such potential is a good thing? Don't know.

The name "bottleneck" implies that every call will eventually execute, like emptying water from a bottle in real life. That seems to imply it wouldn't be possible to bloat bottleneck with additional options related to throttling or debouncing (that is, without grossly conflating the term "bottleneck").

But perhaps discoverability is most important here. 🤔

@radashi-org radashi-org deleted a comment from radashi-bot Aug 18, 2024
@aleclarson aleclarson added the new feature This PR adds a new function or extends an existing one label Aug 22, 2024
@aleclarson aleclarson added this to the v12.3.0 milestone Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature This PR adds a new function or extends an existing one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants